Add browser navigation event#2806
Conversation
|
|
||
| | Attribute | Type | Description | Examples | [Requirement Level](https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/) | Stability | | ||
| |---|---|---|---|---|---| | ||
| | [`browser.navigation.same_document`](/docs/registry/attributes/browser.md) | boolean | A boolean that is true if the navigation is within the same document [1] | `true`; `false` | `Required` |  | |
There was a problem hiding this comment.
question: Any reason to set this attribute as required? I'm worried about browser support here, since the Navigation API is not yet supported in Firefox and Safari.
At the same time, I'm also wondering about the relevance of this attribute: are we expecting the SDK to collect navigation entries related to past documents? Shouldn't those entries already be collected when the past document was active, in which case this attribute would always be true?
There was a problem hiding this comment.
This indicates whether the navigation resulted in loading a new document. I felt it was important to capture because it is the main indicator to specifically distinguish hard page loads.
For Firefox and Safari, we can instrument the History API. Any call to history.pushState or history.replaceState should be captured as same_document = true, while the event captured when the page is first loading should be same_document = false.
There was a problem hiding this comment.
This indicates whether the navigation resulted in loading a new document.
Forgive me if I'm wrong as I am new to the Navigation API, but from my tests it doesn't seem to be the case. sameDocument indicates that the NavigationHistoryEntry was on the current document (also the doc seems to confirm this).
Test:
- load https://example.com/
- in the devtools, look at
navigation.currentEntry.sameDocument. It should betrue(even if it's a whole new document) - Make a soft navigation using
history.pushState(null, '', 'foo')then look atnavigation.entries(). A new navigation entry was added to the list, and both entries havesameDocument: true - Make a hard navigation using
location.href = '/'then look atnavigation.entries()again. A new navigation entry withsameDocument: truewas added to the list, and the first two entriessameDocumentbecamefalse.
There was a problem hiding this comment.
Ooh I understand your point of view as you are referring to the navigateEvent.destination.sameDocument property. In this case yes it distinguishes between soft and hard navigations, but I don't think we should collect hard navigations this way? The hard navigation should be collected after the page is loaded, no?
| | [`browser.navigation.same_document`](/docs/registry/attributes/browser.md) | boolean | A boolean that is true if the navigation is within the same document [1] | `true`; `false` | `Required` |  | | ||
| | [`url.full`](/docs/registry/attributes/url.md) | string | Absolute URL describing a network resource according to [RFC3986](https://www.rfc-editor.org/rfc/rfc3986) [2] | `https://www.foo.bar/search?q=OpenTelemetry#SemConv`; `//localhost` | `Required` |  | | ||
| | [`browser.navigation.hash_change`](/docs/registry/attributes/browser.md) | boolean | A boolean that is true if the navigation is a fragment navigation. [3] | `true`; `false` | `Recommended` |  | | ||
| | [`browser.navigation.type`](/docs/registry/attributes/browser.md) | string | The type of navigation [4] | `push`; `replace`; `reload`; `traverse` | `Recommended` |  | |
There was a problem hiding this comment.
Not sure how much of a benefit we would have in resembling the Navigation event from Browser spec which is basically a History change activity. For Spec, we would be better off capturing this as hard-navigation, soft-navigation, route-change, click etc that triggered the navigations.
I don't think we need to be in 1-1 with Browser Spec.
If we take this route, it would be easier to group all the navigations that are associated within a Session in the backend.
We can also eliminate all the other hashChange, sameDocument fields are the SDK can basically trigger a pageView event dictating whether it was a navigation event that happened due to this type.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 14 days. |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 7 days. |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 7 days. |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 7 days. |
Changes
This adds a browser event for capturing navigation actions.
Proposed here
Merge requirement checklist
[chore]